fix(core): ignore unsafe project provider endpoints - #16
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens hierarchical settings loading to prevent untrusted project-level settings (.coven-code/settings.json{c}) from injecting provider credentials or unsafe endpoints into the effective configuration, while still permitting per-project configuration for known local-only providers.
Changes:
- Sanitize project-loaded
Settingsto strip top-levelconfig.api_key, remove per-providerapi_key, and clearapi_basefor non-allowed providers. - Adjust
Settings::mergeto merge provider configs field-by-field so project settings don’t clobber trusted/global credentials or API bases. - Add unit tests covering provider sanitization and merge behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn sanitize_project_provider_config(provider_id: &str, provider: &mut ProviderConfig) { | ||
| provider.api_key = None; | ||
| if !is_project_api_base_allowed(provider_id) { | ||
| provider.api_base = None; | ||
| } |
| /// Merge two settings with `override_settings` taking priority. | ||
| /// Simple strategy: override wins for all scalar fields; Vecs are | ||
| /// concatenated (deduped); HashMaps are merged (override wins on collision). |
|
Closing this as superseded by #15, merged in #15 strips project-controlled provider routing, provider endpoint maps, provider configs, and config API keys before merge, while preserving safe project preferences and trusted global provider settings. That covers the remaining concern here: local-provider IDs alone are not enough if the project can still point Thanks @BunsDev |
Motivation
Configdoes not clobber global/trusted credentials or bases.Description
SettingsviaSettings::sanitize_project_settings: strip top-levelconfig.api_keyand remove per-providerapi_keyvalues, and clearapi_basefor any provider not explicitly allowed as local (ollama,lmstudio/lm-studio,llamacpp/llama-cpp/llama-server). (changes insrc-rust/crates/core/src/lib.rs)is_project_api_base_allowedandis_default_provider_confighelpers to centralize allowed-local-provider logic and to drop no-op provider entries from project settings. (incore/src/lib.rs)merge_provider_configsforprovider_configs/providersduringSettings::merge. (incore/src/lib.rs)find_project_settingsandmergearepub(crate)) and add unit tests exercising sanitization and merge semantics to prevent regressions. (tests added incore/src/lib.rs)Testing
cargo test --package claurst-core test_project_settings_strip_remote_provider_credentials_and_api_baseand the test passed.cargo test --package claurst-core test_project_provider_sanitization_preserves_global_api_baseandcargo check --package claurst-core, both succeeded.cargo check --workspace,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace) but these are blocked in the current environment due to a missing system dependency (alsa.pc) required byalsa-sysand therefore could not complete here.Codex Task